Skip to content

FIX: Show local-axis grab measurement as a single tilted component#4326

Open
Chetansahney wants to merge 3 commits into
GraphiteEditor:masterfrom
Chetansahney:fix/local-grab-dashed-line
Open

FIX: Show local-axis grab measurement as a single tilted component#4326
Chetansahney wants to merge 3 commits into
GraphiteEditor:masterfrom
Chetansahney:fix/local-grab-dashed-line

Conversation

@Chetansahney

Copy link
Copy Markdown

Closes #3341

When grabbing (G) and pressing X/Y a second time to switch from a global-axis to a local-axis constraint, the on-canvas measurement overlay was drawn incorrectly. It showed an axis-aligned box with both the X and Y screen-space components, and when typing a value it printed that number on both axes, instead of a single line along the tilted local axis.

This implements the change requested in #3527 (comment).

Change

In the Grabbing overlay branch, when the transform is constrained to a single axis in local space, draw one dashed line from the pivot along the (already correctly tilted) translation_viewport vector, with a single measurement label — instead of the screen-aligned translation_box.

The unconstrained and global-axis cases are untouched and still use translation_box. The equivalent overlay for Scale (S) already behaved this way, so this brings Grab in line with it.

When grabbing (G) and pressing X/Y a second time to constrain to a local
axis, the measurement overlay drew an axis-aligned box showing both the X
and Y screen components (and printed a typed value on both axes) instead
of a single line along the tilted local axis.

In the Grabbing overlay branch, when constrained to a single axis in local
space, draw one dashed line along the (already tilted) translation vector
with a single measurement label, mirroring how Scale (S) already behaves.
The unconstrained and global-axis cases are unchanged.

Closes GraphiteEditor#3341
@Chetansahney Chetansahney changed the title Show local-axis grab measurement as a single tilted component FIX: Show local-axis grab measurement as a single tilted component Jul 10, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves the grab transform operation overlay. When translating a layer constrained to a single axis in local space, it now draws a single dashed line with a measurement label along that axis instead of an axis-aligned box. The reviewer suggested reusing the existing format_rounded helper to format the measurement label to maintain consistency and reduce code duplication.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs Outdated
Comment thread editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs Outdated
Comment thread editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs Outdated
Comment thread editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs Outdated
Comment thread editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs Outdated
Chetansahney and others added 2 commits July 18, 2026 12:50
Address review feedback:
- Instead of hand-drawing the dashed line and label in the message
  handler, build the measurement quad in the layer's local frame and let
  the existing translation_box overlay code draw it. This removes the
  duplicated label formatting and the magic text-layout constants.
- Use constraint_axis(..).is_some() as the branch condition rather than
  comparing against Axis::Both.
- Convert the viewport translation to document space with
  document_to_viewport.inverse().transform_vector2() instead of dividing
  by the length of a matrix column.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Chetansahney

Copy link
Copy Markdown
Author

@0HyperCube
Thanks for the review! Pushed e51e597 addressing all the feedback.

The main change: instead of hand-drawing the dashed line and label in the message handler, the measurement quad is now built in the layer's local (tilted) frame and handed to the existing translation_box. Because Quad holds four arbitrary corners and translation_box draws its X edge from top_left to top_right, this renders the single tilted line and one label with no new drawing code. The handler branch went from 13 lines to 3.

That folds in the rest of the feedback as a side effect:

Drawing now lives in translation_box rather than the handler.

The magic constants 16 and 24 are gone, along with the duplicated label formatting, since translation_box already handles both including the typed-string case. This also covers the format_rounded suggestion from gemini-code-assist, as the manual formatting no longer exists.

The branch condition is now constraint_axis(axis_constraint).is_some() && is_transforming_in_local_space, dropping the != Axis::Both comparison and the unwrap_or. I used .is_some() rather than binding the axis, since the vector itself is no longer needed once the quad is built in local space.

The matrix-column division is gone. The label length now comes from document_to_viewport.inverse().transform_vector2(translation_viewport).length(), which is the proper viewport-to-document conversion and stays correct under a non-uniform viewport transform.

Verified locally: cargo check -p graphite-editor passes, cargo clippy reports no warnings in the changed file, and cargo fmt --check is clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local-axis grab (G key) dashed line should show only the X or Y component, tilted

2 participants